📋 Review Summary

This PR addresses a clean separation between internal background operations (suggestion generation, forked queries) and user-visible session logging. The changes prevent internal API calls from appearing in tool call UI, session JSONL recordings, and WebUI replays while preserving token usage tracking for /stats. The implementation is well-structured with consistent patterns across three files.

🔍 General Feedback

• Consistent pattern: The isInternalPromptId() helper is implemented in both LoggingContentGenerator and loggers.ts, providing a clear guard mechanism for internal operations
• Minimal changes: Each modification is surgical and focused on the specific goal of filtering internal operations from persistent logs
• Good preservation: Token usage tracking via _logApiResponse is explicitly preserved for /stats functionality
• Clear documentation: JSDoc comments explain the purpose of the internal prompt ID guard functions

🎯 Specific Feedback

🟡 High

• File: packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts:82 and packages/core/src/telemetry/loggers.ts:130 - Duplicate isInternalPromptId() implementation: The same function is implemented in two separate files. This creates maintenance burden and risk of divergence. Consider extracting this to a shared utility module (e.g., packages/core/src/utils/internalPromptIds.ts) or a common constants file that both modules can import.

  Suggested fix: Create a shared utility:

  // packages/core/src/utils/internalPromptIds.ts
  export function isInternalPromptId(promptId: string): boolean {
    return promptId === 'prompt_suggestion' || promptId === 'forked_query';
  }

🟢 Medium

• File: packages/core/src/followup/forkedQuery.ts:195 - Magic string for tools: []: The comment explains the intent well, but the magic empty array could be extracted to a named constant for clarity and to prevent accidental modification. Consider using Object.freeze([]) or a named constant like NO_TOOLS_CONFIG.

  Suggested improvement:

  const NO_TOOLS_CONFIG: GenerateContentConfig = Object.freeze({ tools: [] });
  const requestConfig: GenerateContentConfig = { ...NO_TOOLS_CONFIG };

• File: packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts:155-165 - Inconsistent null handling for openaiRequest: When isInternal is true, openaiRequest is set to undefined, but the type system may not reflect this properly. The logOpenAIInteraction calls are guarded, which is correct, but consider making the type explicit or using a more explicit pattern.

🔵 Low

• File: packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts:78-83 - JSDoc could mention specific internal operations: The comment mentions "suggestion generation, forked queries" which is good, but could also reference the specific prompt ID values for easier cross-referencing.

• File: packages/core/src/telemetry/loggers.ts:125-130 - Consider exporting isInternalPromptId for testing: If this function needs to be tested in isolation, it should be exported. Currently it's private to the module which may make unit testing the guard logic more difficult.

✅ Highlights

• Excellent test plan: The PR description includes a comprehensive test plan covering all affected functionality (ghost text, suggestion submission, /stats, WebUI replay, speculation feature)
• Clear separation of concerns: The distinction between runForkedQuery (no tools) and createForkedChat (retains tools for speculation) is well-implemented and documented
• Preserves important functionality: Careful attention to preserving _logApiResponse for token tracking while filtering UI-visible logging shows good architectural awareness
• Minimal invasive changes: The guards are placed at the right abstraction level, avoiding the need to thread parameters through multiple layers

已处理 review 反馈，提交 3c28d03fa：

🟡 High — 重复 isInternalPromptId() 实现
已修复。将 isInternalPromptId() 提取为 telemetry/loggers.ts 的 export 函数，LoggingContentGenerator 直接 import 使用，消除重复。同时更新测试 mock 使用 importOriginal 以暴露真实的 isInternalPromptId。

🟢 Medium — tools: [] magic value
保持现状。tools: [] 在 requestConfig 初始化处有清晰注释说明意图，提取为 Object.freeze 常量反而增加间接性，且此处是唯一使用点。

🟢 Medium — openaiRequest null 处理
logOpenAIInteraction 参数类型已为 OpenAI.Chat.ChatCompletionCreateParams | undefined，且第 315 行有 if (!openaiRequest) return 的 early-return guard。当前行为正确。

🔵 Low — JSDoc 和 export
JSDoc 已补充具体 prompt ID 值（prompt_suggestion, forked_query）。函数已改为 export 可用于测试。

已处理维护者反馈，提交 662a123b4：

1. 提取 isInternalPromptId 到共享 utils
已创建 packages/core/src/utils/internalPromptIds.ts，使用 ReadonlySet<string> 管理内部 prompt ID 集合。新增内部 ID 只需改一处。loggers.ts 通过 re-export 保持兼容，loggingContentGenerator.ts 直接从 utils 导入。

2. tools: [] 提取为常量
已在 forkedQuery.ts 中提取为 const NO_TOOLS: GenerateContentConfig = Object.freeze({ tools: [] })，使用处改为 { ...NO_TOOLS }。

3. openaiRequest 类型安全
logOpenAIInteraction 参数类型已为 OpenAI.Chat.ChatCompletionCreateParams | undefined，第 315 行有 if (!openaiRequest) return guard，TypeScript 编译无 warning。

4. 单元测试
已添加 internalPromptIds.test.ts，覆盖：prompt_suggestion → true、forked_query → true、user_query → false、空字符串 → false、其他任意 ID → false。150 个测试全部通过。